@ryzokuken build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/398/pipeline

@addaleax Maybe<bool>::IsTrue is a thing? I didn't know. Also, wouldn't that somehow hurt what we're doing here, as we don't exactly want the same condition to execute? We want it to return to JS-Land if Nothing was returned.

Again, I might be very wrong, but let's look into this.

@ryzokuken value->IsTrue() is a thing, and if we can reasonably expect that value is a boolean already, it’s equivalent to value->BooleanValue(context).ToLocalChecked().

For setAutoPadding, we don’t do any typechecking, but we could make that simplification by using !!ap instead of ap in the JS layer.

For getStringWidth, I’m not sure – they appear unused anyway, beyond a test which accesses internal functions directly (?).

For setNoDelay, we already convert to boolean, so args[0]->IsTrue() is okay to use.

ping @ryzokuken, looks like this PR needs a rebase :)

@maclover7 Will do over the weekend, thanks.

Ping @ryzokuken

Btw there are two other instances of BooleanValue in src, if you also want to change those:

• node_file.cc:

  node/src/node_file.cc

  Line 1463 in e9876fd

  bool with_types = args[2]->BooleanValue();

• tcp_wrap.cc:

  node/src/tcp_wrap.cc

  Line 195 in e9876fd

  bool enable = args[0]->BooleanValue();

I fixed the conflicts and handled the remaining cases.
@addaleax @jasnell @ryzokuken PTAL.

CI: https://ci.nodejs.org/job/node-test-pull-request/16854/

Cannot approve it myself, but LG.

CI is green. Can we have at least 1 LG for my additions?

@addaleax pretty please?

@targos Sorry, missed the pings – yes, this still LGTM :)

Thank you 😃
Landed in 59e5a39

Thanks again, @targos. You're the real MVP.